Feat: Setting as pages #3813
Feat: Setting as pages #3813
Conversation
| @@ -123,3 +123,11 @@ export const workspaceWizardRegionQuery = graphql(` | |||
| } | |||
| } | |||
| `) | |||
|
|
|||
| export const workspaceGetIdBySlugQuery = graphql(` | |||
There was a problem hiding this comment.
Some queries are not possible with workspaceBySlug, as nested properties in some cases need an ID and don't work with slug, so this call is done first to get the id
There was a problem hiding this comment.
There used to be that hack where we just take the workspace's id from the apollo cache, cause it should already be there from the access checks done in middlewares. Can we do that here too?
There was a problem hiding this comment.
Do you remember which hack this was, or maybe @andrewwallacespeckle knows? I vaguely remember it, but dont remember where we are doing this
There was a problem hiding this comment.
I don't know, I think Andrew will know more
There was a problem hiding this comment.
Worked around this by refactoring some of the child components to use workspaceBySlug
packages/frontend-2/components/settings/workspaces/security/sso/Wrapper.vue
Outdated
Show resolved
Hide resolved
packages/frontend-2/pages/settings/workspaces/[slug]/members.vue
Outdated
Show resolved
Hide resolved
packages/frontend-2/pages/settings/workspaces/[slug]/security.vue
Outdated
Show resolved
Hide resolved
packages/frontend-2/pages/settings/workspaces/[slug]/security.vue
Outdated
Show resolved
Hide resolved
packages/frontend-2/nuxt.config.ts
Outdated
| appMiddleware: ['auth', 'settings', 'admin'] | ||
| }, | ||
| '/settings/workspaces/*': { | ||
| appMiddleware: ['auth', 'settings', 'admin', 'requires-workspaces-enabled'] |
There was a problem hiding this comment.
shouldn't these also have the workspace access check middleware?
There was a problem hiding this comment.
Good point, I added it!
| appMiddleware: ['auth', 'settings'] | ||
| }, | ||
| '/settings/server/*': { | ||
| appMiddleware: ['auth', 'settings', 'admin'] |
There was a problem hiding this comment.
hoping these don't get added & invoked twice (cause of the /settings/** rule above)
There was a problem hiding this comment.
It won't, I actually tried that first as I assumed it does, but it will only use one of the routeRules (thats the closest match)
This PR moves away from the settings modal in favor of having them as pages. Some clean-up and bugfixes have been done for the content overall no changes have been made. There is still some room for improvement there, but we can address these in follow-up PRs